fix: attribute registration to the app, not the extension that registered for it (#601) - #604
Closed
jeremydmiller wants to merge 1 commit into
Closed
Conversation
…ered for it (#601) Follow-up to #600 -- same function, different failure mode. Where #600 was about the walk landing on a test runner, this is about it landing on an intermediate Critter Stack assembly. UseWolverine() and AddMarten() both call services.AddJasperFx() from inside their own assembly, so the first frame outside JasperFx belongs to the EXTENSION, not the application. RegistrationCallingAssembly stopped meaning what its name says, and checkForDivergentApplicationAssembly compared the extension against the correctly-resolved application assembly and raised the GH-3521 warning on hosts where nothing was wrong. That inverts the warning's whole value: it trains readers to ignore it, and its absence stops being evidence. On JasperFx/wolverine#3776 "zero occurrences of the warning in the failing logs" was explicitly recorded as grounds for ruling out an application-assembly problem -- which was exactly the bug. Reproduced here rather than taken on faith: a stand-in extension assembly that calls AddJasperFx() on the app's behalf yields RegistrationCallingAssembly=<extension> and ApplicationAssembly=<app>, and the warning FIRES. It is also more than warning noise. The same stack walk seeds the process-wide RememberedApplicationAssembly in AddJasperFx, so wherever IHostEnvironment.ApplicationName is empty the extension assembly became the assembly type discovery scans. Skip Critter Stack framework assemblies in the walk, matched by exact name or dotted prefix so an app named "MartenPlayground" or "WolverineDemo" is left alone, and never matching a *.Tests / *.Testing assembly -- the Critter Stack repos' own suites ARE the application under test, and skipping those would reintroduce #600 from the other side. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Member
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #601.
The defect
Follow-up to #600 — same function, different failure mode. Where #600 was about the walk landing on a test runner, this is about it landing on an intermediate Critter Stack assembly.
UseWolverine()andAddMarten()both callservices.AddJasperFx()from inside their own assembly (Wolverine/HostBuilderExtensions.cs:109,Marten/MartenServiceCollectionExtensions.cs:210,331), so the first frame outside JasperFx belongs to the extension, not the application.Reproduced, not taken on faith
The issue measured Wolverine's twin of this logic and noted JasperFx's own walk had not been instrumented directly. It has now. A stand-in extension assembly that calls
AddJasperFx()on the app's behalf, in a real host:That inverts the warning's whole value. It trains readers to ignore it, and its absence stops being evidence — on JasperFx/wolverine#3776, "zero occurrences of the warning in the failing logs" was explicitly recorded as grounds for ruling out an application-assembly problem, and that was exactly the bug.
It is more than warning noise
The same stack walk seeds the process-wide
RememberedApplicationAssemblyinAddJasperFx. WhereIHostEnvironment.ApplicationNameis empty,establishApplicationAssemblyfalls through to that pin — so the extension assembly became the assembly type discovery scans. Measured on a bareServiceCollection:Note
Warning=nonethere: registered and adopted agree, because both are wrong the same way. The divergence check is silent in exactly the case where it is most needed.The fix
Skip Critter Stack framework assemblies in the walk, alongside the runners:
Two guards against the risk the issue flags — "it must not skip a legitimate application assembly that happens to be named for a product":
StartsWith.WolverineandWolverine.SqlServermatch;WolverineDemo,MartenPlaygroundandJasperFxSamplesdo not.*.Tests/*.Testingassembly.Wolverine.RabbitMQ.TestsandMarten.TestingARE the application as far as discovery is concerned — their handlers and documents are the types under test. Skipping those would reintroduce DetermineCallingAssembly adopts the test-runner assembly, so type discovery scans a runner instead of the test assembly #600 from the other direction.List:
JasperFx·Wolverine·WolverineFx·Marten·Weasel·Polecat·CritterWatch·Oakton.Tests
src/ExtensionStandInis a project whose only purpose is its assembly name —Wolverine.StackWalkStandIn— with an extension method that callsAddJasperFx()on the caller's behalf. Same trick asTestRunnerStandInin #602.registration_is_attributed_to_the_app_not_the_extension_that_registered_for_it— real host; assertsRegistrationCallingAssemblyis the test assembly and that the divergence warning stays quiet.an_extension_registration_does_not_pin_the_extension_as_the_application_assembly— the no-ApplicationNamepath; asserts neitherApplicationAssemblynor the process-wide pin becomes the extension.Both behaviour tests fail without the fix and pass with it. Full
CoreTestsgreen (539 passed, 1 skipped).Not done here
The issue's option 2 — capturing the registering assembly at the outermost public entry point instead of re-deriving it from a stack walk — is the structurally better answer and is unaffected by this change. This is the version that needs no downstream work in Wolverine or Marten to take effect.
🤖 Generated with Claude Code